GH-104 Refactor baltop using database indexes. #104
Conversation
# Conflicts: # eternaleconomy-core/build.gradle.kts
There was a problem hiding this comment.
Code Review
This pull request refactors the leaderboard data flow by extracting the logic from AccountManager into a new LeaderboardServiceImpl. This is a good structural improvement that separates concerns. The new service introduces caching for the top players, which is a great step towards better performance.
However, I've identified several critical performance issues in the new implementation, primarily related to database interactions. The code currently loads large amounts of data (sometimes all accounts) into memory for sorting and filtering, which should be handled by the database using appropriate SQL queries. I've also noted a performance regression in the account prefix search functionality.
My review includes detailed suggestions on how to resolve these performance bottlenecks by leveraging database capabilities more effectively. Additionally, I've pointed out a minor dependency version inconsistency in the build configuration and a suggestion to use proper logging.
...onomy-core/src/main/java/com/eternalcode/economy/account/database/AccountRepositoryImpl.java
Show resolved
Hide resolved
...aleconomy-core/src/main/java/com/eternalcode/economy/leaderboard/LeaderboardServiceImpl.java
Show resolved
Hide resolved
eternaleconomy-core/src/main/java/com/eternalcode/economy/account/AccountManager.java
Outdated
Show resolved
Hide resolved
...aleconomy-core/src/main/java/com/eternalcode/economy/leaderboard/LeaderboardServiceImpl.java
Outdated
Show resolved
Hide resolved
eternaleconomy-core/src/main/java/com/eternalcode/economy/account/AccountManager.java
Show resolved
Hide resolved
…tion for leaderboard and account management. Update dependencies and `AccountManager` to improve consistency.
…ormatting and configuration consistency
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the leaderboard system to improve performance and scalability. The key changes include extracting leaderboard logic from AccountManager into a new LeaderboardServiceImpl, implementing a caching layer with Caffeine, offloading leaderboard queries to the database, and making data retrieval asynchronous. The refactoring is well-executed. However, I've identified a critical regression in the account creation logic and a bug in the new leaderboard implementation. My review includes suggestions to address these points.
eternaleconomy-core/src/main/java/com/eternalcode/economy/account/AccountManager.java
Show resolved
Hide resolved
...aleconomy-core/src/main/java/com/eternalcode/economy/leaderboard/LeaderboardServiceImpl.java
Show resolved
Hide resolved
…rdServiceImpl` to use accurate total entry calculations.
No description provided.